Skip to content

refactor(tests): convert legacy py_extension in local_toolchains to official rule - #3988

Merged
rickeylev merged 22 commits into
bazel-contrib:mainfrom
rickeylev:convert_legacy_py_extension
Aug 8, 2026
Merged

refactor(tests): convert legacy py_extension in local_toolchains to official rule#3988
rickeylev merged 22 commits into
bazel-contrib:mainfrom
rickeylev:convert_legacy_py_extension

Conversation

@rickeylev

Copy link
Copy Markdown
Collaborator

The local_toolchains integration test workspace previously relied on a
custom, partial implementation of py_extension that predated the
official rule.

Replace this legacy test implementation with the official
@rules_python//python/cc:py_extension.bzl rule, simplifying the target
definition to use sources directly.

…fficial rule

The local_toolchains integration test workspace previously relied on a
custom, partial implementation of py_extension that predated the
official rule.

Replace this legacy test implementation with the official
@rules_python//python/cc:py_extension.bzl rule, simplifying the target
definition to use sources directly.
@rickeylev
rickeylev requested a review from aignas as a code owner August 1, 2026 22:14
@rickeylev
rickeylev enabled auto-merge August 1, 2026 22:16
…pansion

When py_extension is invoked from an external module/workspace, expanding  in user_link_flags failed on Windows because the label was not canonicalized to @rules_python//python/cc:current_py_cc_libs.

Use _PY_CC_LIBS_ACTUAL_CANONICAL_TARGET in user_link_flags on Windows.
…ch directories and handle empty link_files

On Windows, when get_local_runtime_info.py finds python3xx.dll in exec_dir, the import library python3xx.lib is typically located in exec_dir/libs. Search across all search_directories for the interface library. Also ensure current_py_cc_libs never outputs an empty DefaultInfo.files so  expansion in py_extension user_link_flags does not crash.
@rickeylev
rickeylev marked this pull request as draft August 1, 2026 22:30
auto-merge was automatically disabled August 1, 2026 22:30

Pull request was converted to draft

@rickeylev

Copy link
Copy Markdown
Collaborator Author

Oh windows...making this difficult again. Lets see how many commits it takes to figure it out this time

…ture Windows local toolchain import libraries

For local Python runtimes on Windows, the CPython interface library (python3xx.lib) is attached to py_cc_toolchain.headers rather than py_cc_toolchain.libs.

By checking the linking_context of both headers and libs in _current_py_cc_libs_impl, current_py_cc_libs properly collects the .lib import library on Windows so MSVC link.exe receives valid import libraries.
@rickeylev
rickeylev marked this pull request as ready for review August 2, 2026 00:23
…eping Windows .lib discovery fix in get_local_runtime_info.py
@rickeylev
rickeylev marked this pull request as draft August 3, 2026 05:50
…inInfo.abi_tag in get_local_runtime_info.py

On POSIX, sysconfig var SOABI contains both ABI and platform tags (e.g. cpython-313-x86_64-linux-gnu). Because py_extension_rule.bzl formats output_filename as {module_name}.{abi_tag}-{platform}.{ext}, passing SOABI directly as abi_tag caused platform tags to be duplicated (.cpython-313-x86_64-linux-gnu-x86_64-linux-gnu.so).

Extracting the ABI portion (e.g. cpython-313) ensures PEP 3149 extension filenames match standard CPython expectations.
…bi3 deps in local_runtime_repo_setup.bzl

On Windows local runtimes, interface_library (python3xx.lib) was added to python_headers, but omitted from libpython. When py_extension_libs extracts Windows link libraries from py_cc_toolchain.libs, it looks at libpython's CcInfo, which was missing interface_library on local runtimes.

Adding :interface and :abi3_interface to libpython deps on Windows ensures py_extension targets on local runtimes find the required .lib files.
…y_extension_rule.bzl

On Windows, CPython extension loading expects {module_name}.{abi_tag}.pyd (e.g. echo_ext.cp313.pyd) rather than POSIX {module_name}.{abi_tag}-{platform}.so (e.g. echo_ext.cp313-win_amd64.pyd).

Formatting Windows output filenames without the platform tag allows Windows CPython to locate and import C extensions properly.
… in py_extension_rule.bzl

Windows CPython extension loader expects module_name.pyd (e.g. echo_ext.pyd) without ABI or platform tags.

Formatting output_filename as module_name.pyd on Windows ensures Windows CPython can locate and import extension modules.
…n echo_test.py

Official py_extension generates PEP 3149 tagged filenames (e.g. echo_ext.cp313-win_amd64.pyd). On Windows, Python's native import statement does not automatically search for PEP 3149 platform-tagged .pyd files during direct import.

Adding an importlib fallback in echo_test.py ensures the extension module is reliably located across all platforms.
Revert temporary importlib fallback in echo_test.py now that upstream py_cc_toolchain soabi resolution handles C extension loading directly.
…ule.bzl

On Windows, CPython's extension module loader checks .cpXX.pyd (e.g. echo_ext.cp311.pyd) or .pyd, rather than POSIX platform-tagged filenames (e.g. .cp311-win_amd64.pyd).

Omitting platform_tag from default soabi generation on Windows ensures py_extension generates .cpXX.pyd filenames that Windows CPython can directly import.
…yd when soabi is empty

On Windows, CPython's extension loader expects unversioned .pyd filenames (e.g. echo_ext.pyd) rather than SOABI-tagged filenames.

Setting default soabi to empty string on Windows and using {module_name}.{ext} when soabi is empty allows Windows CPython to import C extension modules directly.
On Windows, PEP 3149 SOABI tag is cpXX (e.g. cp311), omitting the POSIX platform_tag (-win_amd64).

This ensures py_extension generates output filenames like echo_ext.cp311.pyd, which both satisfies analysis tests expecting 311 in filename and allows Windows CPython to import module directly.
…ension_rule.bzl

On Windows, CPython's extension loader requires unversioned .pyd files (e.g. echo_ext.pyd) without SOABI tags.

Updating py_extension_rule.bzl so output_filename is {module_name}.pyd on Windows ensures Windows CPython can load C extensions directly.
@rickeylev
rickeylev marked this pull request as ready for review August 7, 2026 06:57
@rickeylev
rickeylev enabled auto-merge August 7, 2026 06:57
@rickeylev
rickeylev marked this pull request as draft August 7, 2026 06:59
auto-merge was automatically disabled August 7, 2026 06:59

Pull request was converted to draft

@rickeylev rickeylev added the do not merge Tag that prevents merging label Aug 7, 2026
@rickeylev rickeylev removed the do not merge Tag that prevents merging label Aug 7, 2026
@rickeylev
rickeylev marked this pull request as ready for review August 7, 2026 07:01
@rickeylev
rickeylev enabled auto-merge August 7, 2026 07:01
On Windows with local Python toolchain, loading C extension DLLs requires host PATH to resolve dependent Python DLLs.

Adding env_inherit = ['PYENV_VERSION', 'PATH'] to echo_test ensures Windows test execution inherits host environment PATH.
…_runtime_info.py

On Windows x64, platform.machine() returns uppercase 'AMD64', which caused _get_platform_tag() to fail to match ('x86_64', 'amd64') and fall back to 'win32' instead of 'win_amd64'.

Normalizing platform_machine with .lower() ensures 64-bit Windows generates the expected 'win_amd64' platform tag.
…tform.machine()

Add comment documenting that on Windows platform.machine() returns uppercase 'AMD64' / 'ARM64', while PEP 508 and toolchain tag matching expect lowercase.
@rickeylev
rickeylev added this pull request to the merge queue Aug 8, 2026
Merged via the queue into bazel-contrib:main with commit caa22bd Aug 8, 2026
6 checks passed
@rickeylev
rickeylev deleted the convert_legacy_py_extension branch August 8, 2026 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants